Skip to content

fix(conversions): clean []map[any]any slices before JSON encoding - #193

Merged
hanzei merged 2 commits into
stretchr:masterfrom
mazkev:fix/clean-interface-map-slice
Nov 2, 2026
Merged

fix(conversions): clean []map[any]any slices before JSON encoding#193
hanzei merged 2 commits into
stretchr:masterfrom
mazkev:fix/clean-interface-map-slice

Conversation

@mazkev

@mazkev mazkev commented Oct 30, 2026 •
edited by hanzei
Loading

Copy link
Copy Markdown
Contributor

Summary

  1. Clean []map[any]any slices before JSON encoding:

    • cleanUp(v any) previously supported []any, []map[string]any, []Map, and map[any]any, but omitted []map[any]any (slices of interface maps, e.g. from typed fields filled by YAML decoders or from dynamically built data).
    • Because concrete slice types do not match []any in Go type switches, []map[any]any fell through to default: return v, resulting in json: unsupported type: map[interface {}]interface {} when calling JSON() or Base64().
    • Added cleanUpInterfaceMapArray(in []map[any]any) []Map and registered case []map[any]any: in cleanUp.

  2. Fix stale doc comment:

    • The doc comment on the unexported urlValuesSliceKeySuffix variable still used its old exported name, URLValuesSliceKeySuffix, from before it was made private in URLQuery with index slice keys #74.

  3. Tests:

    • Added unit test cases for nil, empty, and populated []map[any]any in conversions_nil_test.go, and a nested case in TestConversionJSON in conversions_test.go.
    • Verified that all unit tests pass (go test -v ./...).

hanzei
hanzei requested changes Oct 30, 2026
View reviewed changes

@hanzei hanzei left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Can you please explain why URLValuesSliceKeySuffix needs to be exported? What value does it add for callers?

@mazkev

mazkev commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Hi @hanzei, thanks for the review!

The motivation for adding the URLValuesSliceKeySuffix() getter was twofold:

  1. Safe configuration restore / inspection: Since SetURLValuesSliceKeySuffix mutates package-level global state, callers (and tests) currently have no way to inspect the active suffix or temporarily change it and restore the previous state without hardcoding the default constant (e.g. defer SetURLValuesSliceKeySuffix(URLValuesSliceKeySuffix())).
  2. Documentation alignment: The doc comment on lines 17-23 historically referenced URLValuesSliceKeySuffix, which was formerly an exported variable before being made private with a setter.

That being said, if you prefer keeping the public API surface minimal and not exposing the getter, I am more than happy to remove URLValuesSliceKeySuffix() from this PR and keep only the []map[any]any slice cleanup fix!

Let me know what you prefer and I can update the branch accordingly.

hanzei
hanzei requested changes Nov 1, 2026
View reviewed changes

@hanzei hanzei left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the context. you're right that it was exported briefly in 2018 another PR made it private, so the comment has just been stale since then. That said, I'd prefer to keep the API surface as is: callers that change the suffix already know what they set it to, and save/restore is mostly a test concern. Please drop the URLValuesSliceKeySuffix() getter and the test changes, but keep the doc comment rename. The []map[any]any fix looks good and we can merge once that's done.

@mazkev

mazkev commented Nov 1, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @hanzei! I've dropped the URLValuesSliceKeySuffix() getter and reverted the corresponding test changes while keeping the doc comment fix and the []map[any]any slice cleanup with its tests.

Branch is updated and ready for merge!

@hanzei hanzei changed the title fix(conversions): clean []map[any]any slices and add URLValuesSliceKeySuffix getter fix(conversions): clean []map[any]any slices before JSON encoding Nov 2, 2026
@hanzei
hanzei merged commit 0ad041f into stretchr:master Nov 2, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hanzei hanzei hanzei approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mazkev @hanzei